Update Temperature Measurement gen/ folder from ZAP #3826
Conversation
pullapprove
bot
requested review from
andy31415,
BroderickCarlin,
bzbarsky-apple,
chrisdecenzo,
hawk248,
jelderton,
mspang,
rwalker-apple and
saurabhst
| @@ -1,4 +1,5 @@ | |||
| /** | |||
There was a problem hiding this comment.
I wonder whether it makes sense to put these structs in the same order as Simplicity Studio does... would make these changes a lot easier to review.
I'm taking it on faith, for now, that all these struct definitions are correct.
There was a problem hiding this comment.
You should probably not take it on faith.
If you look at it twice you may noticed that there are some empty structs. For example:
With ZAP templates:
// Struct for Signature
typedef struct _Signature
{
} EmberAfSignature;
With AppBuilder:
// Void typedef for EphemeralData which is empty.
// this will result in all the references to the data being as uint8_t*
typedef uint8_t EphemeralData;
It does work because those are not used. I will open an issue for them.
| @@ -120,3 +102,19 @@ EmberAfStatus emberAfBasicClusterServerCommandParse(EmberAfClusterCommand * cmd) | |||
| } | |||
| return status(wasHandled, true, cmd->mfgSpecific); | |||
| } | |||
| EmberAfStatus emberAfTemperatureMeasurementClusterServerCommandParse(EmberAfClusterCommand * cmd) | |||
There was a problem hiding this comment.
Hmm, this ends up the same as the old code, but with more indirection.... I guess that's ok.
There was a problem hiding this comment.
That's a lack of good way to express that with the templates helper. I will open an issue to track that.
| @@ -71,8 +113,8 @@ void emberAfAddToCurrentAppTasksCallback(EmberAfApplicationTask tasks) {} | |||
| * @param value Ver.: always | |||
| * @param type Ver.: always | |||
| */ | |||
| EmberAfAttributeWritePermission emberAfAllowNetworkWriteAttributeCallback(uint8_t endpoint, EmberAfClusterId clusterId, | |||
There was a problem hiding this comment.
For future example apps, we should do the renaming like this in a separate PR (ideally one generated via sed or equivalent). Maybe we should go ahead and do that now-ish, in fact. That will make it clearer which things the ZAP change is really changing.
|
@vivien-apple can you address the merge conflicts? Should be good to go after that! |
Thanks. I'm waiting a little bit, secretly hoping that #3846 lands before ;) If it does not land soon not I will rebase this one. |
vivien-apple
force-pushed
the
ZAP_TemperatureMeasurementUpdate
branch
from
2d21d6e to
354b9c6
Compare
vivien-apple
force-pushed
the
ZAP_TemperatureMeasurementUpdate
branch
from
354b9c6 to
8421ee5
Compare
Problem
#3464 is trying to update all the examples in once. This PR upgrade only the
temperature-measurement-app.In order to produce it, I did a fork with some changes to the ZAP templates (#3780, #3809, #3810 and #3824) and with an extra change to
src/app/util/util.cpp(#3825) as I forgot to add a#ifdefat some point.In order to land, the only requirement from those 5 PRs is #3825.
Then I ran:
node src-script/zap-generate.js -z ./zcl-builtin/silabs/zcl.json -g ../../app/zap-templates/chip-templates.json -i temperature.zap -o ../../../examples/temperature-measurement-app/esp32/main/gentemperature.zapis a slightly modified version from the version in #3464 since the version in #3464 kills the commands of the Basic Cluster.It will require a bit more additional work for the other examples app, notably because the current template for
gen_config.his missing some#define, but in the case of thetemperature-measurement-appenough changes has landed inside CHIP that we can start to use the template from ZAP.Summary of Changes
gen/files from thetemperature-measurement-app